Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make weighted_median iterative #604

Closed
wants to merge 1 commit into from
Closed

Make weighted_median iterative #604

wants to merge 1 commit into from

Conversation

keithtensor
Copy link
Contributor

@keithtensor keithtensor commented Jul 1, 2024

Devnet companion: #628

This PR removes the recursive algorithm in weighted_median and replaces it with a iterative one.

Copy link
Contributor

@gztensor gztensor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing unit tests cause a lot of recursion on both branches of if statement in the older version. I am confident this fix doesn't break it.

Copy link
Contributor

@sam0x17 sam0x17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me!

@sam0x17 sam0x17 added devnet-skip This PR will skip testing on devnet and removed devnet-skip This PR will skip testing on devnet labels Jul 9, 2024
@unconst unconst closed this Jul 23, 2024
@keithtensor keithtensor deleted the end-recursion branch August 20, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants